Allow `const W<dyn A> -> const dyn A` ptr cast #136127

WaffleLapkin · 2025-01-27T12:12:37Z

Followup of #120248 (comment).

This PR allows casting pointers from something wrapping a trait object, to the trait object, i.e. *const W<dyn A> -> *const dyn A where W is struct W<T: ?Sized>(T);.

r? compiler-errors

Fixes #128625

WaffleLapkin · 2025-01-27T12:13:49Z

Implementation wise I feel like I'm not using the trait solver APIs correctly and all this feels quite hack-y. I'd be glad to hear suggestions on how to improve it.

WaffleLapkin · 2025-01-27T12:18:47Z

For the language team nomination: I feel like this is an easy decision. We already support the opposite cast *const dyn A -> *const W<dyn A>, casts where the target is not directly a trait object like *const W<W<dyn A>> -> *const W<dyn A>, and even the *const W<dyn A> -> *const dyn A cast that this PR allows is possible if you persuade the compiler enough:

impl<T: ?Sized> Wrapper<T> {
    fn get(this: *const Self) -> *const T {
        this as _
    }
}

fn cast(ptr: *const Wrapper<dyn Super>) -> *const dyn Super {
    Wrapper::get(ptr)
}

So I'd argue that the reason we don't support *const W<dyn A> -> *const dyn A currently is a compiler bug/oversight (the compiler tries to do unsizing even though it can never succeed, but a pointer cast can).

compiler/rustc_hir_typeck/src/coercion.rs

compiler-errors · 2025-01-27T19:49:07Z

compiler/rustc_hir_typeck/src/coercion.rs

+                            .structurally_normalize(ty, &mut *fulfill_cx)
+                    };
+
+                    // N.B. use `target`, not `coerce_target` (the latter is a var)


Can you clarify in a comment what this is doign? also, i think the "kind match + normalize projection + check its adt kind" can be uplifted into a closure, not just the normalize part.

er, actually maybe just move that comment from above down here?

WaffleLapkin · 2025-02-01T17:06:35Z

@rustbot ready

traviscross · 2025-02-05T16:47:47Z

cc @rust-lang/types

compiler-errors · 2025-02-05T16:48:24Z

pls ping me @WaffleLapkin when this is over, I'll give it one more careful review once the fcp is done.

nikomatsakis · 2025-02-05T16:48:28Z

To confirm, @WaffleLapkin, the intent of this PR is to say "you can cast between two wide raw pointers so long as the metadata is the same vtable in both", roughly speaking?

I take it that the logic is consistency -- i.e., we allow you to convert *const T to *const U for any two thin types T and U, so we already allow the "data pointer" to be arbitrary changed. In this case, the metadata is known to be the same, so you are saying why not let the data ponter be arbitrarily changed, is that correct?

bors · 2025-03-05T19:06:59Z

☔ The latest upstream changes (presumably #138058) made this pull request unmergeable. Please resolve the merge conflicts.

the errors should not be there, this is a bug/missing feature.

this prevents us from trying unsizing coercion in cases like `*const W<dyn T>` -> `*const dyn T`, where it would later cause a compilation error since `W<dyn T>: Sized` and `W<dyn T>: T` do not hold.

yay, I fixed the bug/missing feature :')

compiler-errors · 2025-03-08T19:12:41Z

@bors r+ rollup

bors · 2025-03-08T19:12:43Z

📌 Commit 80157a5 has been approved by compiler-errors

It is now in the queue for this repository.

compiler-errors · 2025-03-08T19:12:59Z

er,

@bors rollup=maybe

…r=compiler-errors Allow `*const W<dyn A> -> *const dyn A` ptr cast Followup of rust-lang#120248 (comment). This PR allows casting pointers from something wrapping a trait object, to the trait object, i.e. `*const W<dyn A> -> *const dyn A` where `W` is `struct W<T: ?Sized>(T);`. r? compiler-errors Fixes rust-lang#128625

Rollup of 16 pull requests Successful merges: - rust-lang#122790 (Apply dllimport in ThinLTO) - rust-lang#136127 (Allow `*const W<dyn A> -> *const dyn A` ptr cast) - rust-lang#136968 (Turn order dependent trait objects future incompat warning into a hard error) - rust-lang#137147 (Add exclude to config.toml) - rust-lang#137319 (Stabilize `const_vec_string_slice`) - rust-lang#137885 (tidy: add triagebot checks) - rust-lang#138040 (compiler: Use `size_of` from the prelude instead of imported) - rust-lang#138052 (strip `-Wlinker-messages` wrappers from `rust-lld` rmake test) - rust-lang#138084 (Use workspace lints for crates in `compiler/`) - rust-lang#138158 (Move more layouting logic to `rustc_abi`) - rust-lang#138160 (depend more on attr_data_structures and move find_attr! there) - rust-lang#138192 (crashes: couple more tests) - rust-lang#138216 (bootstrap: Fix stack printing when a step cycle is detected) - rust-lang#138232 (Reduce verbosity of GCC build log) - rust-lang#138233 (Windows: Don't link std (and run-make) against advapi32, except on win7) - rust-lang#138242 (Revert "Don't test new error messages with the stage 0 compiler") r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 12 pull requests Successful merges: - rust-lang#136127 (Allow `*const W<dyn A> -> *const dyn A` ptr cast) - rust-lang#136968 (Turn order dependent trait objects future incompat warning into a hard error) - rust-lang#137319 (Stabilize `const_vec_string_slice`) - rust-lang#137885 (tidy: add triagebot checks) - rust-lang#138040 (compiler: Use `size_of` from the prelude instead of imported) - rust-lang#138084 (Use workspace lints for crates in `compiler/`) - rust-lang#138158 (Move more layouting logic to `rustc_abi`) - rust-lang#138160 (depend more on attr_data_structures and move find_attr! there) - rust-lang#138192 (crashes: couple more tests) - rust-lang#138216 (bootstrap: Fix stack printing when a step cycle is detected) - rust-lang#138232 (Reduce verbosity of GCC build log) - rust-lang#138242 (Revert "Don't test new error messages with the stage 0 compiler") r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#136127 - WaffleLapkin:dyn_ptr_unwrap_cast, r=compiler-errors Allow `*const W<dyn A> -> *const dyn A` ptr cast Followup of rust-lang#120248 (comment). This PR allows casting pointers from something wrapping a trait object, to the trait object, i.e. `*const W<dyn A> -> *const dyn A` where `W` is `struct W<T: ?Sized>(T);`. r? compiler-errors Fixes rust-lang#128625

Kobzol · 2025-03-09T20:28:09Z

@rust-timer build 63be0e2

rust-timer · 2025-03-09T21:43:49Z

Finished benchmarking commit (63be0e2): comparison URL.

Overall result: ❌ regressions - please read the text below

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	0.7%	[0.1%, 1.6%]	101
Regressions ❌ (secondary)	3.4%	[0.1%, 12.5%]	94
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.7%	[0.1%, 1.6%]	101

Max RSS (memory usage)

Results (primary 1.4%, secondary 2.7%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.8%	[0.6%, 4.0%]	15
Regressions ❌ (secondary)	3.0%	[1.0%, 5.9%]	15
Improvements ✅ (primary)	-4.7%	[-4.7%, -4.7%]	1
Improvements ✅ (secondary)	-2.4%	[-2.4%, -2.4%]	1
All ❌✅ (primary)	1.4%	[-4.7%, 4.0%]	16

Cycles

Results (primary 1.5%, secondary 4.8%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.5%	[0.8%, 4.8%]	23
Regressions ❌ (secondary)	4.8%	[1.3%, 9.2%]	40
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.5%	[0.8%, 4.8%]	23

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 765.417s -> 767.302s (0.25%)
Artifact size: 362.05 MiB -> 362.01 MiB (-0.01%)

Kobzol · 2025-03-09T22:14:38Z

This PR seems to have caused a rather large perf. regression. Do you think it could be avoided somehow? This feature doesn't sound that complex that it would have to cause such perf. hits. Thanks!

compiler-errors · 2025-03-09T22:16:32Z

Can u open an issue @Kobzol to track it? I am on my phone but I can take a look at how to fix this perf regression later or maybe brainstorm w Waffle.

edit: nvm i just cross-linked the pr. probably no tracking issue needed.

…r=<try> Use struct tail rather than metadata projection to compute coercion fast path cc rust-lang#136127 (comment) r? `@ghost`

WaffleLapkin added the relnotes Marks issues that should be documented in the release notes of the next release. label Jan 27, 2025

rustbot mentioned this pull request Jan 27, 2025

Tracking issue for release notes of #136127: Allow *const W<dyn A> -> *const dyn A ptr cast #136128

Open

3 tasks

rustbot assigned compiler-errors Jan 27, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 27, 2025

WaffleLapkin added the I-lang-easy-decision Issue: The decision needed by the team is conjectured to be easy; this does not imply nomination label Jan 27, 2025

This comment has been minimized.

Sign in to view

compiler-errors added the I-lang-nominated Nominated for discussion during a lang team meeting. label Jan 27, 2025

compiler-errors reviewed Jan 27, 2025

View reviewed changes

This comment was marked as resolved.

Sign in to view

WaffleLapkin force-pushed the dyn_ptr_unwrap_cast branch from 5183b33 to 804a940 Compare February 1, 2025 17:01

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 1, 2025

traviscross added T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 5, 2025

This comment was marked as resolved.

Sign in to view

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Feb 5, 2025

This comment was marked as resolved.

Sign in to view

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 5, 2025

WaffleLapkin force-pushed the dyn_ptr_unwrap_cast branch from e620d98 to 3b28c69 Compare March 8, 2025 13:00

WaffleLapkin added 3 commits March 8, 2025 14:49

add a test for pointer casts involving un/re/wrapping trait objects

5712d2e

the errors should not be there, this is a bug/missing feature.

check that UnsizeCoerce may hold before trying unsizing coercion

b62d58f

this prevents us from trying unsizing coercion in cases like `*const W<dyn T>` -> `*const dyn T`, where it would later cause a compilation error since `W<dyn T>: Sized` and `W<dyn T>: T` do not hold.

bless tests

80157a5

yay, I fixed the bug/missing feature :')

WaffleLapkin force-pushed the dyn_ptr_unwrap_cast branch from 3b28c69 to 80157a5 Compare March 8, 2025 13:54

compiler-errors approved these changes Mar 8, 2025

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 8, 2025

jieyouxu mentioned this pull request Mar 9, 2025

Rollup of 16 pull requests #138255

Closed

matthiaskrgr mentioned this pull request Mar 9, 2025

Rollup of 12 pull requests #138267

Merged

bors merged commit 84c2050 into rust-lang:master Mar 9, 2025
6 checks passed

rustbot added this to the 1.87.0 milestone Mar 9, 2025

This comment has been minimized.

Sign in to view

rustbot added the perf-regression Performance regression. label Mar 9, 2025

compiler-errors mentioned this pull request Mar 9, 2025

Use struct tail rather than metadata projection to compute coercion fast path #138292

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow `const W<dyn A> -> const dyn A` ptr cast #136127

Allow `const W<dyn A> -> const dyn A` ptr cast #136127

WaffleLapkin commented Jan 27, 2025 •

edited

Loading

WaffleLapkin commented Jan 27, 2025

WaffleLapkin commented Jan 27, 2025

This comment has been minimized.

compiler-errors Jan 27, 2025

compiler-errors Jan 27, 2025

This comment was marked as resolved.

WaffleLapkin commented Feb 1, 2025

This comment was marked as resolved.

This comment was marked as resolved.

traviscross commented Feb 5, 2025

compiler-errors commented Feb 5, 2025

nikomatsakis commented Feb 5, 2025

This comment was marked as resolved.

bors commented Mar 5, 2025

compiler-errors commented Mar 8, 2025

bors commented Mar 8, 2025

compiler-errors commented Mar 8, 2025

Kobzol commented Mar 9, 2025

This comment has been minimized.

rust-timer commented Mar 9, 2025

Kobzol commented Mar 9, 2025 •

edited

Loading

compiler-errors commented Mar 9, 2025 •

edited

Loading

Allow *const W<dyn A> -> *const dyn A ptr cast #136127

Allow *const W<dyn A> -> *const dyn A ptr cast #136127

Conversation

WaffleLapkin commented Jan 27, 2025 • edited Loading

WaffleLapkin commented Jan 27, 2025

WaffleLapkin commented Jan 27, 2025

This comment has been minimized.

compiler-errors Jan 27, 2025

Choose a reason for hiding this comment

compiler-errors Jan 27, 2025

Choose a reason for hiding this comment

This comment was marked as resolved.

WaffleLapkin commented Feb 1, 2025

This comment was marked as resolved.

This comment was marked as resolved.

traviscross commented Feb 5, 2025

compiler-errors commented Feb 5, 2025

nikomatsakis commented Feb 5, 2025

This comment was marked as resolved.

bors commented Mar 5, 2025

compiler-errors commented Mar 8, 2025

bors commented Mar 8, 2025

compiler-errors commented Mar 8, 2025

Kobzol commented Mar 9, 2025

This comment has been minimized.

rust-timer commented Mar 9, 2025

Overall result: ❌ regressions - please read the text below

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Kobzol commented Mar 9, 2025 • edited Loading

compiler-errors commented Mar 9, 2025 • edited Loading

Allow `const W<dyn A> -> const dyn A` ptr cast #136127

Allow `const W<dyn A> -> const dyn A` ptr cast #136127

WaffleLapkin commented Jan 27, 2025 •

edited

Loading

Kobzol commented Mar 9, 2025 •

edited

Loading

compiler-errors commented Mar 9, 2025 •

edited

Loading